Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix get_python_constraint_from_marker #347

Merged

Conversation

radoering
Copy link
Member

@radoering radoering commented May 8, 2022

  • Added tests for changed code.
  • Updated documentation for changed code.

Fixes get_python_constraint_from_marker() for markers which are not in disjunctive normal form (DNF).

E.g. without the fix:

(python_version < "2.7" or python_full_version >= "3.0.0") and python_full_version < "3.6.0" results in python_constraint *
whereas <2.7 || >=3.0,<3.6 would be correct.

By the way, increased the test coverage for convert_markers() and get_python_constraint_from_marker().

Considering the commits separately may facilitate the review.

Update:

There was another bug in get_python_constraint_from_marker() that is fixed by the last commit.
E.g. for python_version >= "3.9" or sys_platform == "linux" the function returned >=3.9 whereas * would be correct (because for linux all python versions are allowed).

Further, relying on the marker being in DNF before conversion, allows to simplify convert_markers() significantly.

@radoering radoering requested a review from a team May 8, 2022 15:59
@radoering radoering force-pushed the fix-get-python-constraint-from-marker branch from fa94733 to 17169c6 Compare May 8, 2022 16:07
@abn abn mentioned this pull request May 8, 2022
@abn
Copy link
Member

abn commented May 9, 2022

@radoering python-poetry/poetry#5579

src/poetry/core/packages/utils/utils.py Outdated Show resolved Hide resolved
src/poetry/core/packages/dependency.py Outdated Show resolved Hide resolved
…or markers whose disjunctive normal form (DNF) contains a group without marker name "python_version"

By the way, convert_markers() was simplified significantly. That was possible because markers are converted to DNF before conversion.
@radoering radoering force-pushed the fix-get-python-constraint-from-marker branch from a4a4abb to 1183112 Compare May 9, 2022 18:16
@sonarcloud
Copy link

sonarcloud bot commented May 9, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

No Coverage information No Coverage information
1.6% 1.6% Duplication

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants